Wire authorization into workspace model by derekwaynecarr · Pull Request #2445 · NVIDIA/OpenShell

derekwaynecarr · 2026-07-23T23:15:00Z

Summary

Implement RFC 0011 Phase 2 workspace-scoped authorization enforcement. Every gRPC method now carries a proto-level AuthorizationRule annotation that declares its auth mode, required scope, and minimum role. A two-layer authorization architecture enforces these rules: middleware validates authentication mode, scope claims, and global roles; handlers call authorize_workspace() to enforce workspace membership and workspace-level roles. Comprehensive e2e tests cover non-member rejection across ~40 RPCs, cross-workspace isolation, workspace admin privilege boundaries, and the OIDC PKCE flow.

Related Issue

Implements Phase 2 of RFC 0011 — Multi-Player Design

Changes

Proto-driven authorization metadata

Add AuthorizationRule message in proto/options.proto (extension 50000 on MethodOptions) with fields: auth_mode, workspace_role, global_role, scope
Annotate every RPC in openshell.proto and inference.proto with its authorization rule
Auth modes: unauthenticated, bearer, sandbox, dual (bearer + sandbox)
Workspace roles: user, admin; global role: platform_admin

Descriptor pool authorization loader (`descriptor_authz.rs`)

Replace the #[rpc_authz] proc macro with runtime resolution via prost_reflect::DescriptorPool
Build HashMap<String, DescriptorAuthEntry> keyed by gRPC path at startup
Fail-closed: error if any method in openshell.v1 or openshell.inference.v1 is missing its annotation

Middleware authorization (`authz.rs`, `method_authz.rs`)

Rewrite AuthzPolicy::check() to look up proto annotations from the descriptor pool
Methods with global_role require the admin OIDC claim; all others require the user claim
Admin role implicitly satisfies user role requirements
Scope enforcement gated by scopes_enabled flag

Workspace-scoped authorization (`workspace_authz.rs`)

authorize_workspace() — single authorization entry point for all workspace-scoped handlers; checks membership and minimum workspace role, with platform admin bypass
authorize_sandbox_workspace() — data-plane variant that reads workspace from the sandbox record
require_platform_admin() — cross-workspace operations (CreateWorkspace, DeleteWorkspace, etc.)
is_platform_admin_principal() — predicate for conditional logic (e.g. all_workspaces flag)

Handler call sites

Sandbox: CreateSandbox, GetSandbox, ListSandboxes, DeleteSandbox, ExecSandbox, ForwardTcp, WatchSandbox — membership + User role; all_workspaces requires platform admin
Provider: CreateProvider, UpdateProvider, DeleteProvider, ImportProviderProfiles, UpdateProviderProfiles, DeleteProviderProfile, ConfigureProviderRefresh, RotateProviderCredential, DeleteProviderRefresh — membership + Admin role; ListProviders, GetProvider, ListProviderProfiles — membership + User role
Policy: GetSandboxConfig (dual), UpdateConfig (dual — Admin for users, User for sandbox callers with restricted writes), GetDraftPolicy (dual), draft chunk management — membership + Admin role
Inference: SetInferenceRoute, DeleteInferenceRoute — membership + Admin role; GetInferenceRoute — membership + User role
Workspace: GetWorkspace, ListWorkspaceMembers — membership + User role; AddWorkspaceMember, RemoveWorkspaceMember — membership + Admin role; privilege escalation guard prevents non-platform-admins from assigning admin role
ListWorkspaces: Any user with openshell-user OIDC claim can call; handler filters results by membership (platform admins see all, regular users see only workspaces they belong to)

CLI `whoami` command

Add openshell whoami to report the gateway-validated identity (subject, roles, provider)

TUI workspace-scoped provider profiles

Fix providers_v2_enabled to always enable workspace-scoped provider profile fetching (was incorrectly reading a non-existent proto field)

OIDC test infrastructure

Add e2e/with-keycloak.sh — helper script for running commands against a local Keycloak fixture
Add OIDC mode to e2e/with-podman-gateway.sh — conditional mTLS vs OIDC gateway configuration
Add audience mappers to both Keycloak clients in scripts/keycloak-realm.json
Add e2e-oidc-pkce feature and oidc_pkce test target in e2e/rust/
Add e2e/python/oidc/helpers.py — shared OIDC token acquisition and workspace management helpers

E2E test coverage

Python workspace_authz_test.py (~1200 lines): parametrized non-member rejection for ~40 RPCs, dual-mode RPC access, ListWorkspaces membership filtering, platform admin bypass, user member read/write boundary, workspace admin operations, privilege escalation guard, all_workspaces rejection for non-admins
Rust oidc_pkce.rs (~1500 lines): three-identity scenarios (admin, user, user-b); cross-workspace isolation tests for reads, member management, provider management, sandbox creation, and sandbox listing; workspace admin lifecycle scenarios
E2E teardown fix: delete_workspace retries on "still contains resources" to handle async container teardown lag across all runtimes

Testing

mise run pre-commit passes
Unit tests added/updated — descriptor_authz.rs, workspace_authz.rs, method_authz.rs cover all principal types, role combinations, and edge cases (19 tests)
E2E tests added/updated:
- mise run e2e:oidc-pkce (Podman) — 42 passed
- mise run e2e:oidc-pkce:docker (Docker) — 42 passed
- mise run e2e:oidc-python:docker (Docker) — 84 passed

Checklist

Follows Conventional Commits
Commits are signed off (DCO)
Architecture docs updated (if applicable)

copy-pr-bot · 2026-07-23T23:15:04Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

lbelyaev · 2026-07-27T17:18:11Z

This is the piece we've been waiting on, and the two-layer split — proto-annotated middleware for mode/scope/global-role, authorize_workspace() for membership and workspace role. Three notes from running a workspace-scoped authorization model in production, two concrete and one structural.

Membership + role gates who can act, but there's still no workspace-scoped deny. Every call site here resolves to an allow decision: a member with the required role passes, everyone else is rejected. What a Workspace Admin still can't express is a scoped deny — "this workspace may not reach endpoint X," composed under the gateway default rather than replacing it. In our experience that gap is the one enterprises hit first, because their baseline is a deny list they need to hold regardless of what a member is otherwise allowed to do. Phase 2 is the natural place to decide whether that's in scope or explicitly deferred, since the annotation model would have to carry it. Right now it's neither expressible nor called out.
Membership records are keyed by name, so name reuse can rebind authority. authorize_workspace() resolves membership via get_message_by_name(&workspace, &subject), and the id UUID on ObjectMeta isn't part of the authorization key. That means deleting a workspace and later creating a new one with the same name inherits any membership records that were keyed to the old name — a member of the old workspace is silently a member of the new one. This is the follow-up flagged in feat(workspace): add workspace resource model with scoping, membershi… #2243 ("worth revisiting before Phase 2 makes membership records authorization data") — Phase 2 is that moment. Tying the membership record to the workspace's UUID rather than its name solves it.
On dual-mode writes: for UpdateConfig, a sandbox caller authorizes at User and passes through validate_sandbox_caller_update, while a human authorizes at Admin. That split is right. One structural note: validate_sandbox_caller_update is a denylist — it rejects global, delete_setting, empty name, and non-policy-sync shapes, then falls through to Ok. That means a field added to UpdateConfigRequest later is sandbox-writable by default, unless whoever adds it remembers to extend this guard. We've found the inverse — an allowlist of the fields a sandbox caller may touch, defaulting new fields to admin-only — holds up better as the request surface grows, since the failure mode is a rejected write rather than a silently-permitted one.

drew

PR Review Status

Validation: This maintainer-authored PR is project-valid for OpenShell because it implements RFC 0011 Phase 2 workspace-scoped authorization.
Head SHA: a7878d43325a2f6a323c844f3d8b48f688e870eb

Review findings:

Three warning-level findings require author follow-up before gator can move this to pipeline watch.

Docs: Fern docs were updated for the direct workspace/auth UX changes, and folder navigation should pick up the new workspace page.

Next state: gator:in-review

drew · 2026-07-28T15:13:46Z

/ok to test a7878d4

derekwaynecarr · 2026-07-28T17:42:01Z

/ok to test 31c749a

github-actions · 2026-07-28T18:02:11Z

Label test:e2e applied for 31c749a. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

drew · 2026-07-28T18:02:25Z

PR Review Status

Validation: This maintainer-authored PR is project-valid for OpenShell because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI/docs/test surfaces.
Head SHA: 31c749a4903455bb0fb2bde0ca3e0a4f2ee53333

Thanks @derekwaynecarr. I re-checked the fixes you noted on July 28 for descriptor authorization validation and ListWorkspaces pagination; both are resolved on this head. The prior Docker OIDC readiness issue is also resolved by the /healthz readiness path.

Review findings:

No blocking code-review findings remain from the independent principal-engineer review.

Docs: Fern docs were updated for the direct workspace/auth UX changes, including the workspace access page and whoami references. The sandboxes docs folder is navigation-driven, so the new page is picked up by frontmatter ordering.

Checks: OpenShell / Branch Checks is currently failing because cargo fmt --all -- --check reports formatting diffs in crates/openshell-server/src/auth/descriptor_authz.rs in both Rust jobs.

E2E: I applied test:e2e because this PR changes authorization, policy, and sandbox-visible gateway behavior. The E2E label help workflow is still pending.

Next action: @derekwaynecarr, please format crates/openshell-server/src/auth/descriptor_authz.rs and push an update so gator can re-check the new head.

Next state: gator:in-review

derekwaynecarr · 2026-07-28T18:46:17Z

/ok to test 9a7deb9

drew

PR Review Status

Validation: This maintainer-authored PR is project-valid for OpenShell because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI/docs/test surfaces.
Head SHA: 9a7deb96b6c11e438cbe669508e9ad313de272bb

Review findings:

One critical no-auth gateway compatibility regression and one warning-level resource-consumption issue require author follow-up before gator can move this to pipeline watch.

Docs: Fern docs were updated for the direct workspace/auth UX changes, including the new workspace access page under the existing sandboxes navigation folder.

Checks: Required checks are still pending on this head, and test:e2e is applied. Code review findings are currently blocking pipeline handoff.

Next action: @derekwaynecarr, please address the inline gator findings and push an update so gator can re-review the new head.

Next state: gator:in-review

derekwaynecarr · 2026-07-28T19:15:47Z

/ok to test 6bf2335

drew

PR Review Status

Validation: This maintainer-authored PR is project-valid for OpenShell because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI, docs, and E2E surfaces.
Head SHA: 6bf2335cddead5a802aeb87034fa0a1eef9142ae

Thanks @derekwaynecarr. I re-reviewed the latest head after your updates. The descriptor validation, ListWorkspaces pagination correctness, Docker OIDC readiness, and formatting issues from earlier gator passes are no longer the gating items, but the no-auth gateway compatibility issue still remains and the current diff has two additional authorization/resource concerns.

Review findings:

One critical finding and two warning-level findings require author follow-up before gator can move this to pipeline watch.

Docs: Fern docs were updated for the direct workspace/auth UX changes, including workspace access and whoami coverage.

Checks: OpenShell / Branch Checks and OpenShell / Helm Lint are passing on this head. OpenShell / E2E is still pending with test:e2e applied.

Next action: @derekwaynecarr, please address the inline gator findings and push an update so gator can re-review the new head.

Next state: gator:in-review

drew

PR Review Status

Validation: This maintainer-authored PR remains project-valid for OpenShell because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI, docs, and E2E surfaces.
Head SHA: 372d046ddd04b806d4bf0f4d3b1369b4cff7d745

Thanks @derekwaynecarr. I re-reviewed the latest head after your July 28 update. The no-auth gateway compatibility regression, sandbox workspace-enumeration ordering, and unbounded ListWorkspaces membership query from the prior gator pass are improved or resolved on this head. Two warning-level issues still need follow-up before gator can move this to pipeline watch.

Review findings:

Two warning-level findings require author follow-up before gator can move this to pipeline watch.

Docs: Fern docs and the openshell-cli skill were updated for the direct workspace/auth and whoami UX changes.

Checks: OpenShell / Branch Checks, OpenShell / Helm Lint, and OpenShell / E2E are currently waiting for the copy-pr /ok to test mirror on this head. test:e2e remains applied because this PR changes authorization, policy, and sandbox-visible gateway behavior.

Next action: @derekwaynecarr, please address the inline gator findings and push an update so gator can re-review the new head.

Next state: gator:in-review

drew · 2026-07-28T20:26:39Z

/ok to test 372d046

derekwaynecarr · 2026-07-28T21:24:19Z

/ok to test cf2601f

drew

PR Review Status

Validation: This maintainer-authored PR remains project-valid for OpenShell because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI, docs, and E2E surfaces.
Head SHA: cf2601fe3fa1ea7416edd686726154690b3507f6

Thanks @derekwaynecarr. I re-reviewed the latest head after your July 28 updates. The prior no-auth gateway compatibility issue, sandbox workspace-enumeration ordering, and several earlier authorization/pagination findings look improved or resolved on this head. Three warning-level issues still need follow-up before gator can move this to pipeline watch.

Review findings:

Three warning-level findings require author follow-up before gator can move this to pipeline watch.

Docs: Fern docs and the openshell-cli skill were updated for the direct workspace/auth and whoami UX changes.

Checks: OpenShell / Branch Checks and OpenShell / E2E are currently pending on this head, while OpenShell / Helm Lint is passing. test:e2e remains applied because this PR changes authorization, policy, and sandbox-visible gateway behavior.

Next action: @derekwaynecarr, please address the inline gator findings and push an update so gator can re-review the new head.

Next state: gator:in-review

derekwaynecarr · 2026-07-29T16:31:02Z

/ok to test 3b5a656

drew

PR Review Status

Validation: This maintainer-authored PR remains project-valid because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI, TUI, docs, and E2E surfaces.
Head SHA: 3b5a65639fb468d584ed6fa6064d86b6890ee02e

Thanks @derekwaynecarr. I reviewed the new head after your latest authorization, SQLite selector, TUI capability-refresh, and E2E updates. The prior Platform Admin gate on GetGatewayConfig, selector quote handling, and gateway-switch refresh concerns are improved, but the independent review found five line-specific warnings and two general warnings that still need follow-up.

General findings:

Warning (CWE-400): crates/openshell-server/src/grpc/validation.rs:635 and crates/openshell-server/src/persistence/sqlite.rs:537 bound individual label keys and values but not total selector bytes or pair count. SQLite emits one predicate and bind variable per pair. Cap selector size and pair count before persistence and test the boundary.
Warning: .agents/skills/openshell-cli/cli-reference.md:29 still omits whoami from its Complete Command Tree, while .agents/skills/tui-development/SKILL.md:343 and its gateway-switch lifecycle still describe the older sandbox/log-only architecture. Update those companion references for workspace/provider/settings refresh and the new capability fetch.

Docs: Fern docs cover workspace roles, membership, whoami, authorization failures, and platform-only operations. The new page participates in folder-based navigation. The companion agent references above remain stale.

Checks: OpenShell / Branch Checks and OpenShell / E2E are pending. OpenShell / Helm Lint and the non-applicable GPU gate are green. Code-review findings currently block pipeline handoff.

Next action: @derekwaynecarr, please address the inline and general findings and push an update so gator can review the new head.

Next state: gator:in-review

- Cap label selector pairs at 64 (CWE-400) to bound SQLite dynamic SQL - Add SCOPE_ONLY_METHODS allowlist for scope-without-role RPCs (CWE-863) - Normalize ID-based data-plane handlers to return NOT_FOUND for unauthorized sandboxes, closing the cross-workspace oracle (CWE-203) - Fix TUI provider profile cache lookup key mismatch for legacy providers with empty profile_workspace - Add whoami to CLI skill reference command tree - Update TUI skill doc with workspace, provider, and settings coverage - Document scope/workspace orthogonality on GetGatewayConfig proto Signed-off-by: Derek Carr <decarr@redhat.com>

derekwaynecarr · 2026-07-29T20:11:41Z

/ok to test d8f54e0

drew

PR Review Status

Validation: This maintainer-authored PR remains project-valid because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI, TUI, Fern docs, and E2E surfaces.
Head SHA: d8f54e0ff8acc0dfdbfec6b3329f2c8fc20875e0

Thanks @derekwaynecarr. I checked the three prior threads you marked resolved on this head. The broader Platform Admin gate, descriptor validation, SQLite selector, provider-cache, gateway-refresh, docs, and agent-reference issues are fixed, but three narrower behaviors remain unresolved.

Review findings:

Three warning-level findings require follow-up; see the inline comments.

Docs: Fern documentation covers whoami, workspace membership and roles, authorization failures, platform-only operations, and workspace-scoped workflows. The new workspace page participates in the existing folder-driven navigation.

Checks: OpenShell / Branch Checks and OpenShell / E2E are pending on this head. OpenShell / Helm Lint is green, and test:e2e remains applied.

Next action: @derekwaynecarr, please address the inline findings and push an update so gator can re-review the new head.

Next state: gator:in-review

GetSandboxConfig and GetSandboxLogs in policy.rs had the same fetch-before-authorize pattern that leaked cross-workspace sandbox existence. Promote fetch_and_authorize_sandbox to pub(super) and use it from both sandbox.rs and policy.rs handlers. Signed-off-by: Derek Carr <decarr@redhat.com>

derekwaynecarr · 2026-07-29T20:31:00Z

/ok to test 542b259

Cross-workspace sandbox access via ID-based handlers now returns NOT_FOUND instead of PERMISSION_DENIED to prevent existence inference. Update the unit test and OIDC e2e assertion to match. Signed-off-by: Derek Carr <decarr@redhat.com>

derekwaynecarr · 2026-07-29T20:49:53Z

/ok to test baf9ced

drew

PR Review Status

Validation: This maintainer-authored PR remains project-valid because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI, TUI, Fern docs, agent references, and E2E surfaces.
Head SHA: 542b25917ed58eba10fbade4b5f24ad1f3291e43

Thanks @derekwaynecarr. I honored your current-head maintainer dispositions: the GetGatewayConfig scope behavior remains accepted as pre-existing design, and the separate global-settings read-only rendering concern remains waived as follow-up UX polish. I checked the latest diff and complete durable-feedback ledger; the findings below are distinct new regressions or newly delegated trust-boundary risks.

Review findings:

Two critical CWE-918 findings and two warning-level correctness findings require author follow-up; see the inline comments.

General findings:

Warning: crates/openshell-tui/src/app.rs:1151, crates/openshell-tui/src/lib.rs:2101, and crates/openshell-tui/src/lib.rs:2481 still cycle every user into an all workspaces view, but this PR restricts those list calls to Platform Admins. On denial the refreshes retain the prior workspace rows while the title says all. Suppress that option for non-admins, or revert and clear state on denial, and cover an OIDC Workspace User cycle.
Warning: docs/sandboxes/inference-routing.mdx:20,28, .agents/skills/openshell-cli/SKILL.md:509, and .agents/skills/debug-inference/SKILL.md:45 still describe gateway-wide inference and omit workspace selection plus the User-read/Admin-write boundary. Update the Fern page and both maintenance-map companion skills with workspace-scoped commands and denial diagnostics.

Docs: The new workspace Fern page is discoverable through existing folder navigation, but the inference documentation and companion skills above remain stale.

Checks: OpenShell / Branch Checks and OpenShell / E2E are still pending on this head; DCO and OpenShell / Helm Lint are green. Code-review findings currently block pipeline handoff.

Next action: @derekwaynecarr, please address the inline and general findings and push an update so gator can review the new head.

Next state: gator:in-review

…mats Only remap PERMISSION_DENIED to NOT_FOUND in fetch_and_authorize_sandbox and RevokeSshSession, letting INTERNAL and UNAUTHENTICATED propagate as-is. Fix whoami --output format values in cli-reference.md to match the actual CLI (table/json/yaml, not text/json). Signed-off-by: Derek Carr <decarr@redhat.com>

derekwaynecarr · 2026-07-29T21:13:57Z

/ok to test 595195c

drew

PR Review Status

Validation: This maintainer-authored PR remains project-valid because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI, TUI, Fern docs, agent references, and E2E surfaces.
Head SHA: 595195c70bc848f4bace816bfaad2679b009d051

Thanks @derekwaynecarr. I checked your latest head after the sandbox error-mapping, whoami reference, and cross-workspace log-test updates. Those prior findings are resolved, and the independent review found no critical issues. One line-specific authentication-test warning and two general warnings still need follow-up.

General findings:

Warning: crates/openshell-tui/src/app.rs:1151, crates/openshell-tui/src/lib.rs:2101, and crates/openshell-tui/src/lib.rs:2481 still let every user cycle into all workspaces, although those list requests now require Platform Admin. On denial, the title changes to all while prior workspace rows remain displayed. Suppress this option for non-admins, or revert the selection and clear stale rows on denial, and add Workspace User coverage.
Warning: docs/sandboxes/inference-routing.mdx:20, .agents/skills/debug-inference/SKILL.md:18, and .agents/skills/openshell-cli/SKILL.md:509 still describe inference routing as gateway-wide. Update them with workspace selection, the Workspace User read/Workspace Admin write boundary, and permission-denial diagnostics.

Docs: Workspace and whoami Fern documentation is present and folder navigation needs no docs/index.yml change, but the inference-routing documentation and companion skills above remain stale.

Checks: OpenShell / Branch Checks, OpenShell / Helm Lint, and OpenShell / E2E are pending on this head. test:e2e remains applied.

Next action: @derekwaynecarr, please address the inline and general findings and push an update so gator can re-review the new head.

Next state: gator:in-review

derekwaynecarr · 2026-07-29T22:05:35Z

/ok to test e492518

drew

PR Review Status

Validation: This maintainer-authored PR remains project-valid because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI, TUI, Fern docs, agent references, and E2E surfaces.
Head SHA: e492518c89c3f32f7032d4627a4be439b9bcd721

Thanks @derekwaynecarr. I carried forward your July 29 maintainer dispositions on the existing endpoint-fetch hardening concerns. The only change since the last gator-reviewed head is the Keycloak CI network-namespace fix, which does not materially change those code paths, so those waived findings remain suppressed.

Review findings:

Two warning-level findings require follow-up; see the inline comments.

Docs: Fern documentation covers whoami, workspace membership and roles, authorization failures, platform-only operations, and workspace-scoped workflows. The new workspace page participates in the existing folder-driven navigation.

Checks: OpenShell / Branch Checks and OpenShell / E2E are pending on this head. OpenShell / Helm Lint is green, and test:e2e remains applied.

Next action: @derekwaynecarr, please address the inline findings and push an update so gator can re-review the new head.

Next state: gator:in-review

In GitHub Actions job containers, Docker port publishing lands on the host, not inside the job container. Detect this environment and attach Keycloak to the job container's network namespace instead, with hardened defaults (cap-drop ALL, no-new-privileges, loopback-only listener). Signed-off-by: Derek Carr <decarr@redhat.com>

derekwaynecarr · 2026-07-29T22:39:12Z

/ok to test 0e43f1c

drew

PR Review Status

Validation: This maintainer-authored PR remains project-valid because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI, TUI, Fern docs, agent references, and E2E surfaces.
Head SHA: 0e43f1ce9cd9c9e3ad019f3da138c0c4c654a116

Thanks @derekwaynecarr. I reviewed the latest Keycloak network-namespace CI fix and carried forward the complete durable-feedback ledger. That focused CI change does not resolve the two outstanding authentication/input-boundary warnings from the prior head. The independent review also found one distinct SSH-session existence discrepancy; see the inline comments.

Review findings:

Three warning-level findings require follow-up; see the inline comments.

Docs: Fern documentation covers whoami, workspace membership and roles, authorization failures, platform-only operations, and workspace-scoped workflows. The companion CLI/TUI agent references are updated, and the existing folder-driven navigation picks up the new workspace page.

Checks: OpenShell / Branch Checks, OpenShell / Helm Lint, and OpenShell / GPU E2E are green. OpenShell / E2E is pending with test:e2e applied. Code-review findings currently block pipeline handoff.

Next action: @derekwaynecarr, please address the inline findings and push an update so gator can review the new head.

Next state: gator:in-review

Pick up workspace fields from upstream PR NVIDIA#2445 (Wire authorization into workspace model). All request messages now include workspace parameter in the generated Go bindings. Assisted-By: 🤖 Claude Code

…VIDIA#2271) * feat(sdk/go): add Go SDK foundation, types, and sandbox client (A) Add the Go SDK module with the full API contract and a working sandbox client as the first vertical slice. All other resource clients are present as stubs returning Unimplemented errors, to be replaced with real implementations in subsequent PRs. Contents: - Module setup (go.mod, Makefile, mise.toml) - All domain types (types/ package) - Full ClientInterface with all sub-client accessors - Shared infrastructure (errors, auth, gRPC connection, logging) - Sandbox client with converter and tests (fully functional) - Stub clients for remaining resources (exec, file, health, provider, profile, config, refresh, policy, service, ssh, tcp) Part of the Go SDK decomposition plan (NVIDIA#2270). Implements NVIDIA#2044. * fix(sdk/go): address review feedback on PR NVIDIA#2271 - Make scheme parsing drive transport selection: http:// uses plaintext gRPC, https:// or no scheme uses TLS. Add regression tests. - Add Resources and DriverConfig fields to SandboxTemplate and update both converter directions (SandboxFromProto/SandboxSpecToProto). - Regenerate proto bindings from current canonical proto sources to eliminate drift (SigV4/MCP fields, params matchers, reserved fields). - Run gofmt/goimports on all handwritten Go files. Signed-off-by: Roland Huß <rhuss@redhat.com> * fix(sdk/go): address principal engineer review findings - Remove dead boolCount function that would fail golangci-lint (NVIDIA#1) - Emit EventAdded for the first watch event instead of EventModified, matching k8s watch semantics (NVIDIA#7) - Add mutex locking to all mock server methods that access the shared sandboxes map, fixing latent race conditions (NVIDIA#12) - Skip HealthCheck integration test that calls an unimplemented stub (NVIDIA#13) - Scope doc.go examples: mark sections for sub-clients not yet available in this PR with "available in a future release" (NVIDIA#4) - Document Config.Timeout/RetryPolicy/Logger and WatchOptions fields as reserved for future use (NVIDIA#2, NVIDIA#6) Signed-off-by: Roland Huß <rhuss@redhat.com> * refactor(sdk/go): migrate mise config to centralized task include Move Go SDK mise configuration from standalone sdk/go/mise.toml into the project's centralized pattern: - Add Go tools (go, golangci-lint, protoc-gen-go, protoc-gen-go-grpc) to root mise.toml [tools] section - Create tasks/go.toml with all SDK tasks using go: namespace prefix and dir=sdk/go for working directory - Update sdk/go/Makefile to reference namespaced task names - Update proto:sync default path for monorepo layout Addresses review feedback from drew on PR NVIDIA#2271 regarding mise convention alignment. Signed-off-by: Roland Huß <rhuss@redhat.com> * refactor(sdk/go): remove UPSTREAM_VERSION standalone repo artifact Remove sdk/go/proto/UPSTREAM_VERSION file and its exclusion from proto:check. This was a leftover from the standalone repo prototype. In a monorepo, proto drift is detectable via git diff between sdk/go/proto/ and proto/ directly. Signed-off-by: Roland Huß <rhuss@redhat.com> * refactor(sdk/go): switch proto generation from protoc to buf Replace raw protoc invocations with buf for Go SDK proto code generation, aligning with the TS SDK approach (PR NVIDIA#2122). - Add repo-level buf.yaml declaring proto/ as the buf module with lint and breaking change detection config - Add sdk/go/buf.gen.yaml configuring buf to generate Go code directly from root proto/ (no more vendored .proto copies) - Delete vendored .proto source files from sdk/go/proto/ - Rewrite go:proto:gen and go:proto:check mise tasks to use buf - Remove go:proto:sync and go:proto:clean tasks (no longer needed) - Add proto target to sdk/go/Makefile - Add buf 1.72.0 to root mise.toml tool dependencies - Include options.proto in generation (was stripped from vendored copies) - Regenerate all .pb.go files via the new buf pipeline Signed-off-by: Roland Huß <rhuss@redhat.com> * test(sdk/go): add proto-converter field coverage detection Use protobuf reflection to enumerate all fields on key proto messages (SandboxSpec, SandboxTemplate, SandboxStatus, SandboxCondition, SandboxPolicy) and compare against explicit handled/skipped sets in the converter tests. Unhandled fields produce warnings (t.Log), not failures, so proto contributors are not forced to fix SDK converters in the same PR. Stale entries in the handled set (removed proto fields) do fail, since they indicate the converter references something that no longer exists. A follow-up CI workflow will create GitHub issues when converter drift lands on main. Signed-off-by: Roland Huß <rhuss@redhat.com> * fix(sdk/go): bump Go to 1.26 and fix errcheck lint violations The upstream go.mod now has `toolchain go1.26.4`, which requires Go 1.26 to build golangci-lint. Bump the mise.toml Go version from 1.25 to 1.26 and wrap deferred Close() calls in test helpers to satisfy errcheck. Assisted-By: 🤖 Claude Code * feat(sdk/go): add ObjectMeta fields (annotations, workspace, deletion_timestamp) Add three new proto ObjectMeta fields to Sandbox and Provider domain types: Annotations (map), Workspace (string), and DeletionTimestamp (*time.Time). Update converters in both directions, deep-copy maps at the proto/SDK boundary, and add TimeFromMillisPtr/MillisFromTimePtr helper functions. Assisted-By: 🤖 Claude Code * chore(sdk/go): regenerate proto bindings after rebase Pick up workspace fields from upstream PR NVIDIA#2445 (Wire authorization into workspace model). All request messages now include workspace parameter in the generated Go bindings. Assisted-By: 🤖 Claude Code * feat(sdk/go): add workspace scoping to all RPC interfaces Add workspace parameter to every sandbox-scoped RPC method across all interfaces (Sandbox, Exec, File, Service, SSH, TCP, Config, Policy, Provider, Profile, Refresh). The workspace string is passed as the second parameter after ctx, following the convention workspace then resource-name. Key changes: - SandboxInterface: all 10 methods gain workspace parameter - sandbox_client.go: passes Workspace field in every proto request - ListOptions: add AllWorkspaces field for cross-workspace queries - All stub interfaces updated to match new signatures - All sandbox client tests updated with "default" workspace Assisted-By: 🤖 Claude Code * chore(sdk/go): remove coverage.out from tracking Assisted-By: 🤖 Claude Code * fix(sdk/go): address review feedback from mrunalp - Add RefreshStrategyAWSStsAssumeRole to match proto enum value 6, fulfilling the "all domain types upfront" contract - Wrap context.DeadlineExceeded and context.Canceled in StatusError so IsDeadlineExceeded() and IsCancelled() helpers work correctly - Return error from mapToStruct/SandboxSpecToProto instead of silently discarding structpb.NewStruct failures on invalid template maps Signed-off-by: Roland Huss <rhuss@redhat.com> * fix(sdk/go): address remaining review items - Wire go:ci into root ci task so SDK is tested in repository CI - Fix gofmt formatting on converter files - Add goimports to mise.toml tools - Add coverage.out to .gitignore - Add Go SDK section to AGENTS.md and CONTRIBUTING.md - Add regression tests for context-error wrapping (IsDeadlineExceeded, IsCancelled) and invalid template map rejection - Remove panic from SandboxToProto, return error instead Signed-off-by: Roland Huss <rhuss@redhat.com> * fix(sdk/go): pin goimports version and update lockfile Pin goimports to 0.48.0 instead of "latest" and regenerate mise.lock to include the new entry. Signed-off-by: Roland Huss <rhuss@redhat.com> * fix(sdk/go): TLS.Insecure means skip-verify, not plaintext Align TLS.Insecure semantics with the Rust SDK: Insecure: true now uses TLS with InsecureSkipVerify (skip cert verification) instead of switching to plaintext. Only the http:// scheme triggers plaintext. This fixes token auth against dev/k3d gateways: StaticToken and RefreshableToken require transport security, which real TLS (even with InsecureSkipVerify) satisfies, but plaintext does not. For http:// + token auth (dev gateways without TLS), wrap the auth provider to override RequireTransportSecurity, matching the Rust SDK's behavior where http:// accepts any auth mode. Transport decision table (matches Rust SDK crates/openshell-sdk): http:// + any TLS config -> plaintext (TLS config ignored) https:// + Insecure: true -> TLS, skip cert verify https:// + Insecure: false -> TLS, full verification no scheme -> same as https:// Signed-off-by: Roland Huss <rhuss@redhat.com> * feat(sdk/go): add missing policy proto fields Add 6 previously silently dropped fields to the network policy types and converters, preventing security-relevant data loss on round-trip: NetworkEndpoint fields 19-23: - CredentialSigning: SigV4 re-signing mode - SigningService: AWS service name for SigV4 - SigningRegion: AWS region override for SigV4 - JsonRpcMaxBodyBytes: JSON-RPC body inspection limit - Mcp: MCP-specific policy options (new McpOptions type) L7Allow and L7DenyRule field 9: - Params: MCP params matcher map for tools/call filtering New type McpOptions with StrictToolNames and AllowAllKnownMcpMethods optional booleans matching the proto definitions. Signed-off-by: Roland Huss <rhuss@redhat.com> * fix(sdk/go): enforce coverage test and extend to policy messages Change coverage_test.go from t.Logf (silent) to t.Errorf so that unhandled proto fields fail the test immediately. Add coverage tests for NetworkEndpoint (23 fields), L7Allow (8 fields), L7DenyRule (8 fields), and McpOptions (2 fields). Any new proto field that is not in the handled set or explicitly skipped now breaks the build, closing the silent-drift gap. Signed-off-by: Roland Huss <rhuss@redhat.com> * ci(sdk/go): add Go SDK job to branch-checks workflow Add a Go SDK job to branch-checks.yml that runs mise run go:ci (lint, build, test, proto-check, docs-check) on every PR. This ensures the SDK is tested in CI, not just locally. Signed-off-by: Roland Huss <rhuss@redhat.com> * fix(sdk/go): address should-fix review items NVIDIA#6 Fix broken godoc examples: add workspace parameter to all method calls in doc.go that were broken after workspace scoping. NVIDIA#7 Add Err field to Event[T]: Watch error events now carry the underlying error instead of discarding it. NVIDIA#8 Separate Unauthenticated from PermissionDenied: add ErrorUnauthenticated code and IsUnauthenticated() helper. gRPC Unauthenticated (401) now maps to its own code instead of collapsing into PermissionDenied (403). NVIDIA#9 Add Unwrap to StatusError: replace dead Details field with Cause error field. StatusError.Unwrap() returns Cause, enabling errors.Is/As unwrapping. FromGRPCError and contextError both populate Cause. Signed-off-by: Roland Huss <rhuss@redhat.com> * ci(sdk/go): add go:format:check to CI pipeline Add gofmt format verification to go:ci. Catches unformatted Go files before they reach the PR. Fix formatting on coverage_test.go. Signed-off-by: Roland Huss <rhuss@redhat.com> * chore(sdk/go): remove Makefile in favor of mise tasks All build, lint, test, and proto-gen tasks are already defined in tasks/go.toml and invoked via mise. The Makefile was a leftover that duplicated this and raised questions in review. Signed-off-by: Roland Huß <rhuss@redhat.com> * feat(sdk/go): sync proto bindings and add credential handle support Regenerate Go proto bindings after rebase to pick up new CredentialHandle message and Provider.credential_handles and profile_workspace fields from upstream. Add domain types, converter support, and proto field coverage tests for Provider and CredentialHandle. Signed-off-by: Roland Huß <rhuss@redhat.com> * fix(sdk/go): reject plaintext auth leak and fix watch error handling Reject http:// addresses when the auth provider requires transport security instead of silently stripping the requirement. Remove the insecureAuthWrapper that overrode RequireTransportSecurity. Fix watch stream error handling: use blocking send for terminal errors so they are never silently dropped when the channel is full, and wrap mid-stream errors with converter.FromGRPCError so SDK error helpers like IsUnavailable work on watch Event.Err. Signed-off-by: Roland Huß <rhuss@redhat.com> * fix(sdk/go): address review findings from multi-agent code review - WaitReady now detects SandboxDeleting phase and returns immediately instead of polling indefinitely - Watch goroutine defers streamCancel() to prevent context leaks - Fix StopOnTerminal=false test to keep stream open (was wrong-reason pass due to stream ending, not StopOnTerminal logic) - Add EventDeleted test covering the Deleting phase branch - Add provider converter unit tests for CredentialHandle round-trip, nil handling, and empty maps Signed-off-by: Roland Huß <rhuss@redhat.com> --------- Signed-off-by: Roland Huß <rhuss@redhat.com> Signed-off-by: Roland Huss <rhuss@redhat.com>

derekwaynecarr requested review from a team, maxamillion and mrunalp as code owners July 23, 2026 23:15

derekwaynecarr marked this pull request as draft July 23, 2026 23:15

derekwaynecarr force-pushed the decarr/workspace-authz branch from 7b1a5ae to 956cfe9 Compare July 24, 2026 18:20

derekwaynecarr marked this pull request as ready for review July 24, 2026 18:24

lbelyaev mentioned this pull request Jul 27, 2026

feat(kubernetes): map workspaces to sandbox namespaces #2486

Open

2 tasks

derekwaynecarr force-pushed the decarr/workspace-authz branch from 956cfe9 to a7878d4 Compare July 28, 2026 14:58

drew reviewed Jul 28, 2026

View reviewed changes

Comment thread e2e/with-docker-gateway.sh

Comment thread crates/openshell-server/src/grpc/workspace.rs Outdated

Comment thread crates/openshell-server/src/auth/descriptor_authz.rs

drew added the gator:in-review Gator is reviewing or awaiting PR review feedback label Jul 28, 2026

derekwaynecarr force-pushed the decarr/workspace-authz branch from a7878d4 to 31c749a Compare July 28, 2026 17:40

drew added the test:e2e Requires end-to-end coverage label Jul 28, 2026

derekwaynecarr force-pushed the decarr/workspace-authz branch from 31c749a to 9a7deb9 Compare July 28, 2026 18:45

drew reviewed Jul 28, 2026

View reviewed changes

Comment thread crates/openshell-server/src/grpc/mod.rs

Comment thread crates/openshell-server/src/grpc/workspace.rs Outdated

drew reviewed Jul 28, 2026

View reviewed changes

Comment thread crates/openshell-server/src/grpc/mod.rs

Comment thread crates/openshell-server/src/grpc/workspace.rs Outdated

Comment thread crates/openshell-server/src/grpc/sandbox.rs Outdated

drew reviewed Jul 28, 2026

View reviewed changes

Comment thread crates/openshell-server/src/grpc/provider.rs Outdated

Comment thread crates/openshell-server/src/grpc/workspace.rs Outdated

drew reviewed Jul 28, 2026

View reviewed changes

Comment thread crates/openshell-server/src/grpc/provider.rs Outdated

Comment thread crates/openshell-server/src/inference.rs Outdated

Comment thread crates/openshell-server/src/persistence/sqlite.rs Outdated

drew added gator:blocked Gator is blocked by process or repository gates and removed gator:in-review Gator is reviewing or awaiting PR review feedback gator:blocked Gator is blocked by process or repository gates labels Jul 29, 2026